Complexes PR #2: changes to a first set of packages #3479
Conversation
…t conflict with Complexes package
…with Complexes package (InvolutiveBases, Posets, RandomComplexes, SpectralSequences, one Macaulay2Doc doc node)
| @@ -42,8 +42,8 @@ export {--types | |||
| "CellDimension", | |||
| "InferLabels", | |||
| "LabelRing", | |||
| "Reduced", | |||
There was a problem hiding this comment.
Pedantically speaking, removing the comma here is not necessary since export silently ignores null entries.
| @@ -27,10 +27,10 @@ assert(l1=!=l2); | |||
| assert(dim l1==1); | |||
| assert(dim l2==1); | |||
| C = cellComplex(QQ,{l1,l2}); | |||
| CchainComplex = chainComplex C; | |||
There was a problem hiding this comment.
Minor point: I think some of the changed lines here aren't necessary since CchainComplex is just a variable, and since C is a (cell) complex having "chain" in the variable name doesn't hurt.
| if opts.Labels === {} then C[1] | ||
| else C | ||
| ) | ||
| ) | ||
|
|
||
| homology(ZZ, SimplicialComplex, Ring) := Module => opts -> (i, Delta, R) -> ( | ||
| homology(i, chainComplex Delta ** R) | ||
| homology(i, (complex Delta) ** R) |
There was a problem hiding this comment.
Was the parenthesis necessary here or is it just for clarity?
| @@ -3035,7 +3035,7 @@ doc /// | |||
| prune homology Γ | |||
| prune homology(Γ, QQ) | |||
| prune homology(Γ, ZZ/2) | |||
| assert(prune homology(Γ, ZZ/2) == gradedModule((ZZ/2)^2[-1] ++ (ZZ/2)^1[-2])) | |||
| assert(prune homology(Γ, ZZ/2) == (complex (ZZ/2)^2)[-1] ++ (complex (ZZ/2)^1)[-2]) | |||
There was a problem hiding this comment.
If the plan is to entirely remove gradedModule, and change M[n] to return a Complex, this would just become (ZZ/2)^2[-1] ++ (ZZ/2)^1[-2], correct?
|
The changes to SimplicialComplexes look fine to me. |
|
ReactionNetworks change looks ok to me -- in fact, that function should probably be deleted. |
|
@mikestillman do you want to go ahead and merge this, or wait for @jkyang92 or @OlaSobieska and @smapes or @gwynwhieldon to respond first? You can also open the next pull request on top of this (i.e. these commits will also be included in the next one, but you can at least see if there are any tests that need fixing). |
|
@ggsmith and I meet this afternooon, we will discuss how to handle that. |
|
I will merge this pull request. @mahrud @d-torrance I'm still fuzzy about the following: if I do the "rebase and merge" button on github, can I still use same branch for PR's later? |
Yup! The commits that end up on the |
Minor changes to
Posets,InvolutiveBases,SimplicialComplexes,CellularResolutions, andReactionNetworksto get these working with the new Complexes package.@jkyang92 @OlaSobieska: Please look over the minor changes to
CellularResolutions.@smapes @gwynwhieldon: Please look over the minor changes to
Posets.@sashahbc: Please look over the minor changes to
SimplicialComplexes@antonleykin @timduff35 @CvetelinaHill @klee669 @alexandru-iosif @MikeAdamer : Please look over the minor changes to
ReactionNetworks. (I think all we did was to make one unused function local).